[codex] Add Python feature mapping#2
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cc9ed57a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
0cc9ed5 to
99d8316
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65c5339720
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (await pathExists(join(root, "pdm.lock"))) { | ||
| return "pdm"; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Add Hatch runner when forming Python default commands
detectPackageManagers classifies repos with hatch.toml as using the hatch package manager, but pythonRunner only recognizes uv, poetry, and pdm, so defaults like pytest/ruff are emitted without a runner prefix. In Hatch-managed projects where tools are only available inside Hatch environments, these suggested commands will fail even though manager detection succeeded; this makes command detection internally inconsistent for the same repo.
Useful? React with 👍 / 👎.
| const toolMatch = /^\s*\[(mypy|pyright|ruff)\]/mu.exec(source); | ||
| if (toolMatch?.[1] !== undefined) { |
There was a problem hiding this comment.
Parse all setup.cfg tool sections before deriving commands
This reads only the first [mypy|pyright|ruff] section in setup.cfg because exec is used once on the whole file. If [mypy] appears before [ruff] (or vice versa), later tool sections are ignored, which can suppress expected defaults (for example, missing Ruff lint/format commands despite an explicit [ruff] section). Iterating all matches or checking each tool independently avoids this false negative.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43b2147fee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (await pathExists(join(root, "pdm.lock"))) { | ||
| return "pdm run pytest"; | ||
| } | ||
| return "pytest"; |
There was a problem hiding this comment.
Prefix Hatch-managed test commands with hatch run
pythonTestCommand only special-cases uv.lock, poetry.lock, and pdm.lock, then falls back to plain pytest. In Hatch projects (hatch.toml present), detectProject already reports hatch run pytest, but Python feature seeds (CLI/source/test-suite tests fields) will still carry bare pytest, which can fail when pytest is only available inside Hatch environments. Add a Hatch branch here so mapper test commands stay executable and consistent with detected defaults.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ebc27108e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const file of ["requirements.txt", "setup.cfg"]) { | ||
| if (await pathExists(join(root, file))) { | ||
| const source = await readFile(join(root, file), "utf8"); | ||
| for (const dependency of pythonRequirementNames(source)) { | ||
| info.dependencies.add(dependency); |
There was a problem hiding this comment.
Ignore setup.cfg extras keys in dependency detection
The setup.cfg path currently runs every line through pythonRequirementNames, so extras group keys (for example [options.extras_require] entries like ruff = ... or pytest = ...) are treated as installed dependencies even when those packages are not declared. In that case pythonDefaultCommands will incorrectly enable ruff/pytest defaults for projects that only define similarly named extras groups, which violates the intended conservative command detection.
Useful? React with 👍 / 👎.
| async function dependencyFileHas(root: string, dependency: string): Promise<boolean> { | ||
| for (const file of ["requirements.txt", "setup.cfg"]) { | ||
| if (!(await pathExists(join(root, file)))) { | ||
| continue; | ||
| } | ||
| const source = await readFile(join(root, file), "utf8"); | ||
| if (requirementNames(source).has(dependency)) { |
There was a problem hiding this comment.
Stop inferring pytest from setup.cfg group names
The mapper’s dependencyFileHas checks setup.cfg with requirementNames, which also interprets extras group keys as dependencies. A project with [options.extras_require] and a group named pytest will get pytest test commands in mapped features even if pytest is not installed or configured, causing non-executable suggested test commands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6b42d397b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [...dirs].some((dir) => pathMatchesPrefix(test, dir)) || | ||
| (fileStems.has(testStem) && /^(tests?|__tests__)\//u.test(test)) |
There was a problem hiding this comment.
Associate root tests when source file is at repo root
associatedTests misses root-level pytest files for root modules because dirname("app.py") is ".", and pathMatchesPrefix(test, ".") never matches paths like test_app.py; the fallback stem check also requires tests to live under tests?/. In projects with flat layouts and console scripts targeting root modules, this drops obvious colocated tests from the CLI/source feature context.
Useful? React with 👍 / 👎.
| for (const dependency of pythonRequirementNames(source)) { | ||
| info.dependencies.add(dependency); | ||
| } | ||
| if (/^\s*\[tool:pytest\]|\[pytest\]/mu.test(source)) { |
There was a problem hiding this comment.
Anchor pytest section regex to avoid comment false positives
The pytest-config check uses ^\s*\[tool:pytest\]|\[pytest\], where only the first alternative is start-anchored. Any incidental "[pytest]" text (for example in comments or sample snippets) sets hasPytestConfig = true, which can incorrectly enable default pytest commands in projects that are not actually configured for pytest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b93dc9efd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const modulePath = `${moduleName.replace(/\./gu, "/")}.py`; | ||
| const packageInitPath = `${moduleName.replace(/\./gu, "/")}/__init__.py`; |
There was a problem hiding this comment.
Restrict console-script target paths to repo-local modules
resolvePythonScript turns pyproject.toml script targets into file paths by only replacing dots, so targets containing path separators like ../outside or /tmp/mod are preserved and then accepted when pathExists(join(root, candidate)) succeeds. In those cases the mapper emits entryPath/ownedFiles that point outside the repository, which can leak unrelated files into feature mapping and any later automation that trusts mapped paths. Validate targets as import-style module names (or check isSafeFile/pathInsideRoot) before accepting a candidate.
Useful? React with 👍 / 👎.
| .filter(isPythonTestPath) | ||
| .filter((path) => !pythonShouldSkip(path)); |
There was a problem hiding this comment.
Exclude fixture directories when collecting Python test files
pythonTestFiles filters by isPythonTestPath and pythonShouldSkip, but unlike source-file filtering it does not exclude fixtures, __fixtures__, or testdata. Repositories that keep sample pytest files under fixture trees will be mapped as real test suites and associated tests, creating noisy or misleading Python features. Apply the same fixture/testdata exclusion used by isReviewablePythonSourceFile when building nestedTests.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01bd65aeb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (/^test_[^/]+\.py$/u.test(path) || path.endsWith("_test.py")) { | ||
| return "tests"; |
There was a problem hiding this comment.
* Keep nested _test.py suites under their directory
testSuiteRoot currently sends any path ending in _test.py to the synthetic tests root, even when the file is nested (for example src/pkg/foo_test.py). That collapses colocated test files into a repo-level suite, so standalone Python test-suite features lose directory locality and can no longer represent the actual package/module area they validate.
Useful? React with 👍 / 👎.
| table(source, "tool.poetry.dependencies"), | ||
| table(source, "tool.poetry.dev-dependencies"), | ||
| ...tablesMatching(source, /^tool\.poetry\.group\.[^.]+\.dependencies$/u), | ||
| ]) { | ||
| for (const value of assignedKeysAndValues(dependencyTable)) { |
There was a problem hiding this comment.
Avoid treating PDM group names as installed dependencies
Including tool.pdm.dev-dependencies in the assignedKeysAndValues path causes group keys (for example pytest = ["httpx"]) to be interpreted as real dependencies. That makes dependencyNames(source).has("pytest") true even when pytest is not declared in any value, so Python mapping can emit pytest commands for environments where pytest is not actually installed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21109c0568
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| async function pythonTestFiles(root: string): Promise<string[]> { | ||
| const rootTests = await rootPythonTestFiles(root); | ||
| const nestedTests = (await walk(root, ["tests", "test", ...(await pythonSourceRoots(root))])) |
There was a problem hiding this comment.
Include tests when collecting Python test files
pythonTestFiles only walks tests, test, and discovered source roots, but associatedTests explicitly treats __tests__/ as a valid Python test location. In repos that store pytest files under __tests__ (for example __tests__/test_cli.py), no python-test-suite feature is created and source/CLI features lose their test associations even though the mapper is already designed to accept that path prefix.
Useful? React with 👍 / 👎.
| } | ||
| info.hasPytestConfig = info.tools.has("pytest") || info.tools.has("pytest.ini_options"); | ||
| } | ||
| for (const file of ["requirements.txt", "setup.cfg"]) { |
There was a problem hiding this comment.
Detect pytest config from pytest.ini and tox.ini
Pytest detection for default commands only checks pyproject.toml plus requirements.txt/setup.cfg, so projects configured via pytest.ini or tox.ini are missed. If such a project also uses non-default test file patterns and does not declare pytest as a dependency, detectProject returns test: null even though pytest is clearly configured, which suppresses runnable test defaults.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7e23587b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const tool of ["uv", "hatch"]) { | ||
| if (!found.includes(tool) && (await pyprojectHasToolSection(root, tool))) { | ||
| found.push(tool); | ||
| } |
There was a problem hiding this comment.
Detect Poetry/PDM from pyproject tool sections
detectPackageManagers only checks pyproject.toml tool sections for uv and hatch, so projects that declare [tool.poetry] or [tool.pdm] without committed lockfiles fall back to generic python/pip detection. In those repos, command detection emits bare pytest/ruff instead of poetry run ... or pdm run ..., which can be non-runnable when tools exist only in the managed environment. This commit already parses Poetry/PDM tables for dependency inference, so manager inference should include those tool sections as well.
Useful? React with 👍 / 👎.
| for (const sourceRoot of sourceRoots) { | ||
| if (await isSafeDirectory(root, join(root, sourceRoot))) { | ||
| roots.push(sourceRoot); | ||
| } |
There was a problem hiding this comment.
Include repo-root Python modules in source grouping
pythonSourceRoots only includes fixed directories (src, app, apps, lib, scripts) and top-level package directories with __init__.py, but never the repository root itself. As a result, flat-layout modules like main.py/utils.py at the repo root are excluded from pythonSourceGroups, so valid Python projects can miss python-source-group features and lose associated test/context mapping for those files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edfc402f2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (rawValue.startsWith("[")) { | ||
| values.push(...pythonTomlArrayValues(readTomlBracketValue(source, valueStart))); | ||
| continue; | ||
| } | ||
| values.push(...pythonTomlArrayValues(rawValue)); |
There was a problem hiding this comment.
Ignore inline-table string fields in dependency parsing
When a TOML assignment value is an inline table (for example requests = { extras = ["pytest"], version = "^2" } in [tool.poetry.dependencies]), this branch feeds the whole table into pythonTomlArrayValues, which extracts every quoted string and then treats them as package names. That makes extras names like pytest/ruff look like installed dependencies, so detectProject can emit pytest/ruff default commands even though those tools are not declared dependencies.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a30696148
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (rawValue.startsWith("[")) { | ||
| values.push(...arrayValues(readBracketValue(source, valueStart))); | ||
| continue; | ||
| } | ||
| values.push(...arrayValues(rawValue)); |
There was a problem hiding this comment.
Ignore inline-table string fields when inferring dependencies
assignedValues treats any non-array assignment value as a generic string container, so for TOML inline tables (for example requests = { extras = ["pytest"], version = "^2" }) it extracts "pytest" and later dependencyNames interprets it as an installed package. In Python mapping this makes readPyproject(...).hasPytest true even when pytest is only mentioned as an extra name, which can produce non-runnable pytest commands and test associations for projects that do not actually depend on pytest.
Useful? React with 👍 / 👎.
| !pyproject.hasPytest && | ||
| !(await dependencyFileHas(root, "pytest")) && | ||
| (await pythonTestFiles(root)).length === 0 |
There was a problem hiding this comment.
Detect pytest config files before dropping Python test command
This early return only checks pyproject/setup dependency hints and discovered test_*.py files, so repositories configured through pytest.ini or tox.ini (especially with non-default test naming) can end up with testCommand = null even though pytest is explicitly configured. That causes mapped Python features to lose executable test commands and under-represent testability in exactly those config-driven projects.
Useful? React with 👍 / 👎.
Summary
Adds first-pass Python support to clawpatch feature mapping.
This includes:
pyproject.toml, setup files, requirements, and common Python source layoutssrcroots, and colocated testsNotes
Framework-specific Python route mapping for FastAPI, Flask, and Django remains out of scope for this first pass.
Validation
corepack pnpm typecheckcorepack pnpm lintcorepack pnpm format:checkcorepack pnpm test